-
Notifications
You must be signed in to change notification settings - Fork 6
Fix env var detection UX in workato init #26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Refactored environment variable detection to happen before profile selection, providing clear user feedback and explicit confirmation before using env vars. Changes: - Move env var detection from _create_new_profile() to _setup_profile() - Detect WORKATO_API_TOKEN and WORKATO_HOST before showing profile menu - Ask user "Use environment variables for authentication?" with clear messaging - Add _select_profile_name_for_env_vars() to select profile name without credentials - Add _create_profile_with_env_vars() to create profile using env var credentials - Simplify _create_new_profile() by removing env var detection logic - Update non-interactive mode in init.py to use env vars automatically - Remove 5 obsolete tests that tested old env var behavior This fixes the confusing UX where selecting an existing profile would silently use environment variable credentials without informing the user. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for automatically detecting and using WORKATO_API_TOKEN and WORKATO_HOST environment variables during CLI initialization, allowing non-interactive mode to work without explicit CLI flags when these environment variables are set.
Key changes:
- Environment variables are checked and used as fallback values when CLI flags are not provided
- Region detection is enhanced to automatically determine region from
WORKATO_HOSTURL - Interactive mode prompts users to confirm usage of detected environment variables
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/workato_platform_cli/cli/commands/init.py | Added environment variable detection in non-interactive mode and updated error messages to reference env vars |
| src/workato_platform_cli/cli/utils/config/manager.py | Implemented env var detection, region matching from URLs, and new profile creation flow for env vars |
| tests/unit/config/test_manager.py | Added test coverage for non-interactive mode with environment variables and region detection |
Comments suppressed due to low confidence (1)
src/workato_platform_cli/cli/utils/config/manager.py:146
- This statement is unreachable.
api_url = region_info.url
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Remove unreachable dead code that checked api_url after it was already used, and add explicit warning when overwriting existing profiles with environment variable credentials to prevent accidental data loss. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Update non-interactive mode validation to accept either --region or --api-url (along with --api-token). The region detection logic already handles extracting region from api_url, so the validation should reflect this capability. This prevents confusing error messages when users provide --api-url without --region, since the code can auto-detect region from the URL. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Fix duplicate self parameter in test_setup_profile_existing_create_new_success that was accidentally introduced during refactoring. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
oalami
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, did a bunch of manual testing and it working smoothly. @cmworkato please take a pass as well
|
I'm still seeing issues. Test 1: When ENV Vars are set in the current shell, Steps to reproduce:
Output: ✗ env | grep WORKATO (workato-platform-cli) ➜ workato-platform-cli git:(bugfix/detect-env-vars-in-init) ✗ workato workspace |
|
`workato init` works to create a new credential in the keychain, then other
commands run as expected.
…On Mon, Nov 3, 2025 at 12:10 PM Robert Fujara ***@***.***> wrote:
*hovu96* left a comment (workato-devs/workato-platform-cli#26)
<#26 (comment)>
So are you saying that workato init works for you?
(Sounds like you found another issue around workato workspace and maybe
other commands)
—
Reply to this email directly, view it on GitHub
<#26 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BTZPYIOHBFJ4I5D7XKKNHC3326ZBRAVCNFSM6AAAAACKRL22USVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTIOBSGM3TGOJQGE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
This is a pure refactoring with no functional changes to user-facing behavior. Changes: - Replaced ~70 lines of duplicated region selection code in both _create_profile_with_env_vars() and _create_new_profile() methods - Both methods now call the existing ProfileManager.select_region_interactive() helper method instead of duplicating the logic - Updated test mocks to use the helper method instead of mocking inline code Benefits: - Eliminates code duplication (DRY principle) - Adds missing URL security validation (HTTP only allowed for localhost) - Adds URL normalization (strips path components) - Improves maintainability - changes only needed in one place - Net reduction of 54 lines of code Testing: - All 921 tests pass - Updated 2 existing tests to mock the helper method - Manual testing confirms all flows work correctly: * workato init with both WORKATO_API_TOKEN and WORKATO_HOST * workato init with only WORKATO_API_TOKEN (no host) * workato init with no environment variables 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…e setup Add 6 new test cases to verify environment variable detection behavior during profile initialization: - Both WORKATO_API_TOKEN and WORKATO_HOST detected - User declining to use detected env vars - Partial env vars requiring additional input - Existing profile overwrite confirmation flow - Cancellation when user declines overwrite These tests ensure robust handling of environment variables during the init process and verify proper user confirmation flows. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Enhanced the user experience when only partial environment variables are detected (e.g., only WORKATO_API_TOKEN without WORKATO_HOST): - Explicitly show what's missing with clear messages - Context-aware confirmation prompts based on detected variables - Simplified verbose messaging to avoid redundancy - Removed duplicate prompt headers Users now get clear expectations about what additional input is needed without unnecessary repetition or verbose explanations. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Enhanced transparency in non-interactive mode when environment variables are used as fallback for CLI flags: - Regular mode: Shows clear messages when WORKATO_API_TOKEN and/or WORKATO_HOST are used from environment - JSON mode: Maintains valid JSON output without text messages - CLI flags take precedence: No messages shown when explicit flags provided - Interactive mode: Completely unchanged (zero impact) Added comprehensive test coverage: - test_init_non_interactive_with_env_vars_regular_mode - test_init_non_interactive_with_env_vars_json_mode - test_init_non_interactive_cli_flags_take_precedence - test_init_non_interactive_partial_env_vars_json All 931 tests pass. Linters and type checks pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
I tested various scenarios with/without the environment variables WORKATO_HOST and WORKATO_API_TOKEN and based on my tests, everything works as expected. Today, I improved the UX messaging when environment variables are detected (making it clearer what's found/missing) and increased test coverage for these scenarios, but no functional changes around credential resolution. Questions for @cmworkato:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (2)
tests/unit/config/test_manager.py:1
- Removed line appears to be a dictionary key for prompt responses. Verify that the custom URL prompt is no longer needed in this test case or if its removal is intentional. The test still expects custom region handling but may no longer properly mock the URL prompt.
"""Tests for ConfigManager."""
tests/unit/config/test_manager.py:1
- Removed code was mocking
inquirer.promptfor region selection. With the new implementation usingselect_region_interactive(lines 2005-2012), this removal appears correct. However, verify that the test still properly validates custom region behavior.
"""Tests for ConfigManager."""
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Fixes the UX issue where
workato initsilently usedWORKATO_API_TOKENandWORKATO_HOSTenvironment variables without informing the user, causing confusion about which credentials were being used.Changes
Interactive Mode:
Non-Interactive Mode:
Technical Details
_create_new_profile()to_setup_profile()_select_profile_name_for_env_vars()for profile name selection_create_profile_with_env_vars()for creating profiles with env var credentials_create_new_profile()by removing env var detection logicinit.pyTesting
✅ All 905 tests passing
⚠️ Needs thorough manual testing - see DEVP-422 for test scenarios
✅ Basic manual testing completed
Related Issues
🤖 Generated with Claude Code